Skip to content

Check for Version File#447

Merged
ExtremeFiretop merged 2 commits intoExtremeFiretop:devfrom
Martinski4GitHub:dev
Apr 8, 2025
Merged

Check for Version File#447
ExtremeFiretop merged 2 commits intoExtremeFiretop:devfrom
Martinski4GitHub:dev

Conversation

@Martinski4GitHub
Copy link
Collaborator

Added a check for the version.txt file.

@Martinski4GitHub
Copy link
Collaborator Author

@ExtremeFiretop,

Can you re-test on the AiMesh node using this latest PR?

@ExtremeFiretop ExtremeFiretop merged commit a285c69 into ExtremeFiretop:dev Apr 8, 2025
1 check passed
@ExtremeFiretop
Copy link
Owner

@ExtremeFiretop,

Can you re-test on the AiMesh node using this latest PR?

Completed find below results:

image

And:

image

It did add the version.txt. but only after the update check was already completed, it makes the flow feel a little weird
(notified that theres an update available when there isn't, and mentioned the cron jobs twice)

I'm wondering if we moved it up further in the process so it completed before the update check if that would be better.
To refernece; my original set install method worked like this from my PR:

image

@ExtremeFiretop
Copy link
Owner

@Martinski4GitHub

Any value in just reverting back to that method? but keeping your changes for the configuration file?

@ExtremeFiretop
Copy link
Owner

ExtremeFiretop commented Apr 8, 2025

Tell the snow to go away. View from out my window right now. The spring needs to stop teasing us

IMG20250408022423

@Martinski4GitHub
Copy link
Collaborator Author

@ExtremeFiretop,
Can you re-test on the AiMesh node using this latest PR?

Completed find below results:
...
And:
...

It did add the version.txt. but only after the update check was already completed, it makes the flow feel a little weird (notified that theres an update available when there isn't, and mentioned the cron jobs twice)

I'm wondering if we moved it up further in the process so it completed before the update check if that would be better. To refernece; my original set install method worked like this from my PR:

Yes, good point. I've moved up the function call. Can you try again?

@Martinski4GitHub
Copy link
Collaborator Author

@Martinski4GitHub

Any value in just reverting back to that method? but keeping your changes for the configuration file?

The issue with that method, as written, is that it will keep making the "install" parameter call every single time on AiMesh nodes. Why? Because it checks for an entry in the hook file that will never be present on AiMesh nodes.

@ExtremeFiretop
Copy link
Owner

@Martinski4GitHub
Any value in just reverting back to that method? but keeping your changes for the configuration file?

The issue with that method, as written, is that it will keep making the "install" parameter call every single time on AiMesh nodes. Why? Because it checks for an entry in the hook file that will never be present on AiMesh nodes.

On the AiMesh nodes you would have this "#MerlinAU" tag:

image

@ExtremeFiretop
Copy link
Owner

I guess thats dependent on the update checks being enabled though?
Idk it seemed like a working solution for me

@ExtremeFiretop
Copy link
Owner

Oh I see; the tag name would need to be changed for that to match as well.

@Martinski4GitHub
Copy link
Collaborator Author

@Martinski4GitHub
Any value in just reverting back to that method? but keeping your changes for the configuration file?

The issue with that method, as written, is that it will keep making the "install" parameter call every single time on AiMesh nodes. Why? Because it checks for an entry in the hook file that will never be present on AiMesh nodes.

On the AiMesh nodes you would have this "#MerlinAU" tag:

Yeah, but it may not be always there, and the code was looking for the "#MerlinAU#" tag

@Martinski4GitHub
Copy link
Collaborator Author

I guess thats dependent on the update checks being enabled though?

Correct, which would make the method unreliable. I'd rather make the code check for exactly the file(s) that we are looking for and expecting to be installed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants